feat: return status depending on the checker's severity by ktro2828 · Pull Request #216 · tier4/t4-devkit

ktro2828 · 2025-11-11T13:22:42Z

What

This PR is related to #212 (comment).

This pull request introduces a severity level system for sanity checkers, distinguishing between errors and warnings, and updates both the codebase and documentation to reflect these changes. It also updates the reporting format to include warnings and their counts, and revises some rules to be warnings instead of errors.

Key changes include:

Sanity Checker Severity and Reporting:

Introduced a Severity enum (ERROR, WARNING) and updated the Checker base class to support severity levels for rules, affecting how results are reported and handled. (t4_devkit/sanity/checker.py, [1] [2]
Updated all field-type checkers to explicitly set their severity to ERROR. (t4_devkit/sanity/format/fmt001.py, [1] [2]; ...and similar changes in all fmtXXX.py files)
Changed some rules in the requirements schema from Error to Warn to reflect their new severity. (docs/schema/requirement.md, docs/schema/requirement.mdL10-R12)

Documentation and Output Format Updates:

Updated the CLI documentation to show warnings in the summary table and revised the JSON output format to include a severity field and a new WARNING status. (docs/cli/t4sanity.md, [1] [2]

Codebase Consistency and Refactoring:

Refactored imports and class definitions in all field-type checker modules to use the new Severity enum and set the severity explicitly. (All t4_devkit/sanity/format/fmtXXX.py files, e.g., [1] [2] etc.)
Updated the FieldTypeChecker docstring to document the new severity attribute. (t4_devkit/sanity/format/base.py, t4_devkit/sanity/format/base.pyR24)

Usage

There is no change about the usage of t4sanity CLI and sanity_check(...) function, but the output JSON file contains a new severity field as folllows:

{
    "dataset_id": "65785904-4f0a-4e3f-adf7-2935ecdb5db8",
    "version": null,
    "reports": [
        {
            "id": "FMT001",
            "name": "attribute-field",
            "severity": "ERROR",
            "description": "All types of 'Attribute' fields are valid.",
            "status": "SUCCESS",
            "reasons": null
        }
    ]
}

result.json

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

github-actions · 2025-11-11T13:23:16Z

☂️ Python Coverage

current status: ❌

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
3892	2125	55%	50%	🟢

New Files

No new covered files...

Modified Files

File	Coverage	Status
t4_devkit/sanity/checker.py	0%	🔴
t4_devkit/sanity/format/base.py	0%	🔴
t4_devkit/sanity/format/fmt001.py	0%	🔴
t4_devkit/sanity/format/fmt002.py	0%	🔴
t4_devkit/sanity/format/fmt003.py	0%	🔴
t4_devkit/sanity/format/fmt004.py	0%	🔴
t4_devkit/sanity/format/fmt005.py	0%	🔴
t4_devkit/sanity/format/fmt006.py	0%	🔴
t4_devkit/sanity/format/fmt007.py	0%	🔴
t4_devkit/sanity/format/fmt008.py	0%	🔴
t4_devkit/sanity/format/fmt009.py	0%	🔴
t4_devkit/sanity/format/fmt010.py	0%	🔴
t4_devkit/sanity/format/fmt011.py	0%	🔴
t4_devkit/sanity/format/fmt012.py	0%	🔴
t4_devkit/sanity/format/fmt013.py	0%	🔴
t4_devkit/sanity/format/fmt014.py	0%	🔴
t4_devkit/sanity/format/fmt015.py	0%	🔴
t4_devkit/sanity/format/fmt016.py	0%	🔴
t4_devkit/sanity/format/fmt017.py	0%	🔴
t4_devkit/sanity/format/fmt018.py	0%	🔴
t4_devkit/sanity/record/base.py	0%	🔴
t4_devkit/sanity/record/rec001.py	0%	🔴
t4_devkit/sanity/record/rec002.py	0%	🔴
t4_devkit/sanity/record/rec003.py	0%	🔴
t4_devkit/sanity/record/rec004.py	0%	🔴
t4_devkit/sanity/record/rec005.py	0%	🔴
t4_devkit/sanity/record/rec006.py	0%	🔴
t4_devkit/sanity/reference/base.py	0%	🔴
t4_devkit/sanity/reference/ref001.py	0%	🔴
t4_devkit/sanity/reference/ref002.py	0%	🔴
t4_devkit/sanity/reference/ref003.py	0%	🔴
t4_devkit/sanity/reference/ref004.py	0%	🔴
t4_devkit/sanity/reference/ref005.py	0%	🔴
t4_devkit/sanity/reference/ref006.py	0%	🔴
t4_devkit/sanity/reference/ref007.py	0%	🔴
t4_devkit/sanity/reference/ref008.py	0%	🔴
t4_devkit/sanity/reference/ref009.py	0%	🔴
t4_devkit/sanity/reference/ref010.py	0%	🔴
t4_devkit/sanity/reference/ref011.py	0%	🔴
t4_devkit/sanity/reference/ref012.py	0%	🔴
t4_devkit/sanity/reference/ref013.py	0%	🔴
t4_devkit/sanity/reference/ref014.py	0%	🔴
t4_devkit/sanity/reference/ref015.py	0%	🔴
t4_devkit/sanity/result.py	0%	🔴
t4_devkit/sanity/structure/str001.py	0%	🔴
t4_devkit/sanity/structure/str002.py	0%	🔴
t4_devkit/sanity/structure/str003.py	0%	🔴
t4_devkit/sanity/structure/str004.py	0%	🔴
t4_devkit/sanity/structure/str005.py	0%	🔴
t4_devkit/sanity/structure/str006.py	0%	🔴
t4_devkit/sanity/structure/str007.py	0%	🔴
t4_devkit/sanity/structure/str008.py	0%	🔴
t4_devkit/sanity/structure/str009.py	0%	🔴
t4_devkit/sanity/tier4/tiv001.py	0%	🔴
TOTAL	0%	🔴

updated for commit: 88e45ce by action🐍

Copilot

Pull Request Overview

This PR introduces a severity level system for sanity checkers, enabling differentiation between errors and warnings. The implementation adds a Severity enum and updates the Checker base class to support severity-based reporting, where warnings produce WARNING status instead of FAILURE. Several structure validation rules (STR001, STR004-006, STR008-009) are downgraded from errors to warnings.

Key changes:

Added Severity enum with ERROR and WARNING levels
Updated all checkers to explicitly declare their severity level
Modified reporting to display warnings separately from failures
Downgraded optional directory/file checks to warnings

Reviewed Changes

Copilot reviewed 56 out of 56 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
t4_devkit/sanity/checker.py	Added `Severity` enum and updated `Checker` class to route warnings vs failures based on severity
t4_devkit/sanity/result.py	Added `WARNING` status, `make_warning()` function, and updated report display/summary logic
t4_devkit/sanity/structure/str*.py	Updated structure checkers with `Severity` import and explicit severity declarations
t4_devkit/sanity/format/fmt*.py	Updated format checkers to include `Severity` import and `severity = Severity.ERROR`
t4_devkit/sanity/reference/ref*.py	Updated reference checkers with severity declarations
t4_devkit/sanity/record/rec*.py	Updated record checkers with severity declarations
t4_devkit/sanity/tier4/tiv001.py	Added severity declaration and reordered imports
t4_devkit/sanity/*/base.py	Updated docstrings to document new `severity` attribute
docs/schema/requirement.md	Changed STR004, STR005, STR006 from `Error` to `Warn`
docs/cli/t4sanity.md	Updated documentation to show warnings column and new JSON format

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

shekharhimanshu · 2025-11-12T02:39:18Z

-+-----------+---------+---------+-------+---------+----------+-------+
-| DatasetID | Version | Status  | Rules | Success | Failures | Skips |
-+-----------+---------+---------+-------+---------+----------+-------+
-| dataset1  |    0    | SUCCESS |  44   |   44    |    0     |   0   |
-+-----------+---------+---------+-------+---------+----------+-------+
+-----------+---------+---------+-------+---------+----------+----------+-------+
+| DatasetID | Version | Status  | Rules | Success | Warnings | Failures | Skips |
+-----------+---------+---------+-------+---------+----------+----------+-------+
+| dataset1  |         | SUCCESS |  49   |   43    |    4     |    0     |   2   |
+-----------+---------+---------+-------+---------+----------+----------+-------+
 ```

 ### Dump Results as JSON

 To dump results into JSON, use the `-o; --output` option:

 ```shell
 t4sanity <DATA_ROOT> -o result.json
 ```

 Then a JSON file named `result.json` will be generated as follows:

 ```json
 {
  "dataset_id": "<DatasetID: str>",
  "version": <Version: int>,
  "reports": [
    {
        "id": "<RuleID: str>",
        "name": "<RuleName: str>",
+        "severity": "<ERROR/WARNING: str>",
        "description": "<Description: str>",
-        "status": "<SUCCESS/FAILURE/SKIPPED: str>",
+        "status": "<SUCCESS/WARNING/FAILURE/SKIPPED: str>",
        "reasons": "<[<Reason1>, <Reason2>, ...]: [str; N] | null>" // Failure or skipped reasons, null if success
    },


[imho]

Overall design discussion

IMHO, The current design looks more complex than it needs to be.
Right now both severity (ERROR / WARNING) and status (SUCCESS / WARNING / FAILURE / SKIPPED) overlap in meaning, which makes it hard to tell whether “WARNING” refers to rule importance or to the check’s outcome. This also makes options like --include-warning ambiguous — it’s unclear whether they skip execution, hide output, or influence exit codes.

To make the behavior clearer and closer to common linting/validation tools (flake8, pytest, pre-commit), I’d suggest the following adjustments:

Separate “severity” from “status”
• Severity → metadata of rule; defined per rule (WARNING, ERROR)
• Status → result of running the rule (PASSED, FAILED, SKIPPED)
• Example: a WARNING-severity rule can still fail, but that doesn’t necessarily cause the tool to exit with an error.

Clarify CLI flag behavior
• --exclude → skip rule execution (status=SKIPPED)
• --show-warnings (rename from --include-warning) → control output visibility, not execution
• Optionally add --strict or --treat-warnings-as-errors for CI enforcement when warnings should fail the build

Simplify exit status or API logic
Exit Status Logic

Condition --strict / --treat-warnings-as-errors Exit Code Notes

At least one ERROR-severity rule failed N/A 1 Always fails the run

At least one WARNING-severity rule failed, no ERROR failures Off (default) 0 Run is considered successful; warnings are reported

At least one WARNING-severity rule failed, no ERROR failures On 1 Treat warnings as errors; exit with failure

All rules passed or skipped N/A 0 Success

Internal error / invalid CLI usage N/A >1 Indicates tool or usage error

Adjust JSON and summary output

Example result schema:

{ "id": "STR001", "severity": "ERROR", "status": "FAILED", "description": "something is not correct", "reasons": [...] }

Summary table:

Dataset Version Passed Failed Skipped Warnings (count)

Overall, this simplifies the mental model:
• Severity = how important a rule is
• Status = what happened when it ran
• Exit code/API call result = whether any error-level rules failed

That keeps the CLI behavior predictable, reduces confusion, and aligns well with established patterns for sanity/linting tools.

@shekharhimanshu Thank you for detailed suggestions!

Separate “severity” from “status”
• Severity → metadata of rule; defined per rule (WARNING, ERROR)
• Status → result of running the rule (PASSED, FAILED, SKIPPED)
• Example: a WARNING-severity rule can still fail, but that doesn’t necessarily cause the tool to exit with an error.

I agree with you. In cebfaed, I addressed to remove Status.WARNING and return Status.SUCCESS for rules whose Severity == WARNING.

• Optionally add --strict or --treat-warnings-as-errors for CI enforcement when warnings should fail the build

I added -s; --strict option in 29f3e11.
This option enables us to treat rules whose Severity == WARNING as Status.FAILURE if they have warning reasons.

I also agree with your renaming suggestions, but let me address them in the another PR!

Thank you!

I have one concern about this specification:

return Status.SUCCESS for rules whose Severity == WARNING.

since status is just the output of running a rule, status should be independent of severity. Therefore, if a WARNING severity rule fails, the report should have Status.FAILURE.

Severity comes into play to decide the overall result of validation.

Default behavior (strict = false): Validation OK if there are no Status.FAILURE for severity=ERROR rules (Warning with Status.FAILURE are just reported in summary)

strict = true behavior: Validation OK if there are no Status.FAILURE for both severity=ERROR & WARNING rules

This is my understanding of the rules design. Please check 🙏🏽

since status is just the output of running a rule, status should be independent of severity. Therefore, if a WARNING severity rule fails, the report should have Status.FAILURE.
Severity comes into play to decide the overall result of validation.

Default behavior (strict = false): Validation OK if there are no Status.FAILURE for severity=ERROR rules (Warning with Status.FAILURE are just reported in summary)

strict = true behavior: Validation OK if there are no Status.FAILURE for both severity=ERROR & WARNING rules

Thank you, I updated in 3f9a1d3.

without `-s; --strict` option

$ t4sanity ./data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/ === DatasetID: 65785904-4f0a-4e3f-adf7-2935ecdb5db8 === FMT001: ✅ FMT002: ✅ FMT003: ✅ FMT004: ✅ FMT005: ✅ FMT006: ✅ FMT007: ✅ FMT008: ✅ FMT009: ✅ FMT010: ✅ FMT011: ✅ FMT012: ✅ FMT013: ✅ FMT014: ✅ FMT015: ✅ FMT016: ✅ FMT017: ✅ FMT018: ✅ REC001: ✅ REC002: ✅ REC003: ✅ REC004: ✅ REC005: ✅ REC006: ✅ REF001: ✅ REF002: ✅ REF003: ✅ REF004: ✅ REF005: ✅ REF006: ✅ REF007: ✅ REF008: ✅ REF009: ✅ REF010: ✅ REF011: ✅ REF012: [SKIPPED] - Missing lidarseg.json REF013: ✅ REF014: ✅ REF015: [SKIPPED] - Missing data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/annotation/lidarseg.json STR001: - 'version' directory doesn't exist STR002: ✅ STR003: ✅ STR004: - Path to 'map' not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map STR005: ✅ STR006: ✅ STR007: ✅ STR008: - Path to 'map' directory not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map STR009: - Path to 'map' directory not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map TIV001: ✅ +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+ | DatasetID | Version | Status | Rules | Success | Failures | Skips | Warnings | +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+ | 65785904-4f0a-4e3f-adf7-2935ecdb5db8 | | SUCCESS | 49 | 49 | 0 | 2 | 4 | +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+

with `-s; --strict` option

$ t4sanity ./data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/ -s === DatasetID: 65785904-4f0a-4e3f-adf7-2935ecdb5db8 === FMT001: ✅ FMT002: ✅ FMT003: ✅ FMT004: ✅ FMT005: ✅ FMT006: ✅ FMT007: ✅ FMT008: ✅ FMT009: ✅ FMT010: ✅ FMT011: ✅ FMT012: ✅ FMT013: ✅ FMT014: ✅ FMT015: ✅ FMT016: ✅ FMT017: ✅ FMT018: ✅ REC001: ✅ REC002: ✅ REC003: ✅ REC004: ✅ REC005: ✅ REC006: ✅ REF001: ✅ REF002: ✅ REF003: ✅ REF004: ✅ REF005: ✅ REF006: ✅ REF007: ✅ REF008: ✅ REF009: ✅ REF010: ✅ REF011: ✅ REF012: [SKIPPED] - Missing lidarseg.json REF013: ✅ REF014: ✅ REF015: [SKIPPED] - Missing data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/annotation/lidarseg.json STR001: - 'version' directory doesn't exist STR002: ✅ STR003: ✅ STR004: - Path to 'map' not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map STR005: ✅ STR006: ✅ STR007: ✅ STR008: - Path to 'map' directory not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map STR009: - Path to 'map' directory not found: data/annotation_dataset/65785904-4f0a-4e3f-adf7-2935ecdb5db8/map TIV001: ✅ +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+ | DatasetID | Version | Status | Rules | Success | Failures | Skips | Warnings | +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+ | 65785904-4f0a-4e3f-adf7-2935ecdb5db8 | | FAILURE | 49 | 45 | 4 | 2 | 4 | +--------------------------------------+---------+---------+-------+---------+----------+-------+----------+

shekharhimanshu · 2025-11-12T02:40:49Z

@ktro2828
PRありがとうございます！CLIの設計についてご相談で、コメントしました。

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

…failed, but their reports are treated as sucuess unless strict=True Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

shekharhimanshu

LGTM. Thank you! 💯

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

feat: return status depending on the checker's severity

3f5d52b

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

Copilot AI review requested due to automatic review settings November 11, 2025 13:22

github-actions Bot added documentation Improvements or additions to documentation new-feature New feature or request labels Nov 11, 2025

ktro2828 mentioned this pull request Nov 11, 2025

[FEAT] extend sanity checking #211

Closed

Copilot AI reviewed Nov 11, 2025

View reviewed changes

Comment thread t4_devkit/sanity/result.py Outdated

Comment thread t4_devkit/sanity/tier4/tiv001.py

ktro2828 requested review from SamratThapa120 and shekharhimanshu November 11, 2025 13:24

Update t4_devkit/sanity/result.py

c99fbf7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

shekharhimanshu reviewed Nov 12, 2025

View reviewed changes

feat: remove warning from status

cebfaed

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

ktro2828 force-pushed the feat/sanity/severity branch from a7ed208 to c8a6a3f Compare November 12, 2025 06:26

feat: add strict option

29f3e11

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

ktro2828 force-pushed the feat/sanity/severity branch from c8a6a3f to 29f3e11 Compare November 12, 2025 06:30

feat: always set Status.FAILURE if the checker with Severity.WARNING …

3f9a1d3

…failed, but their reports are treated as sucuess unless strict=True Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

shekharhimanshu approved these changes Nov 14, 2025

View reviewed changes

ci: update CLI test

88e45ce

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>

github-actions Bot added the ci Continuous Integration (CI) processes and testing label Nov 14, 2025

ktro2828 merged commit f72c0f6 into main Nov 14, 2025
5 checks passed

ktro2828 deleted the feat/sanity/severity branch November 14, 2025 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: return status depending on the checker's severity#216

feat: return status depending on the checker's severity#216
ktro2828 merged 6 commits into
mainfrom
feat/sanity/severity

ktro2828 commented Nov 11, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 11, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

shekharhimanshu Nov 12, 2025 •

edited

Loading

Uh oh!

ktro2828 Nov 12, 2025 •

edited

Loading

Uh oh!

shekharhimanshu Nov 12, 2025

Uh oh!

ktro2828 Nov 14, 2025

Uh oh!

shekharhimanshu commented Nov 12, 2025

Uh oh!

shekharhimanshu left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Condition	`--strict` / `--treat-warnings-as-errors`	Exit Code	Notes
At least one `ERROR`-severity rule failed	N/A	1	Always fails the run
At least one `WARNING`-severity rule failed, no `ERROR` failures	Off (default)	0	Run is considered successful; warnings are reported
At least one `WARNING`-severity rule failed, no `ERROR` failures	On	1	Treat warnings as errors; exit with failure
All rules passed or skipped	N/A	0	Success
Internal error / invalid CLI usage	N/A	>1	Indicates tool or usage error

Conversation

ktro2828 commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Usage

Uh oh!

github-actions Bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

shekharhimanshu Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktro2828 Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shekharhimanshu Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

ktro2828 Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

shekharhimanshu commented Nov 12, 2025

Uh oh!

shekharhimanshu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ktro2828 commented Nov 11, 2025 •

edited

Loading

github-actions Bot commented Nov 11, 2025 •

edited

Loading

shekharhimanshu Nov 12, 2025 •

edited

Loading

ktro2828 Nov 12, 2025 •

edited

Loading